Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds fonts folder & Work Sans #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Adds fonts folder & Work Sans #5

wants to merge 2 commits into from

Conversation

Shrinks99
Copy link
Member

Fonts are important! This folder gives org members a place to go to get all open source / free fonts that we use.

Work Sans is licensed under the SIL Open Font License and as such they are cool with us redistributing it. The readme has been updated with information regarding fonts in this repo.

@Shrinks99
Adds fonts folder & work-sans
5bcf881 
Fonts are important to design!  This folder gives org members a place to go to get all open source / free fonts that we use.

Work Sans is licensed under the SIL Open Font License and as such they are cool with us redistributing it.  Licenses should be included in their respective font folder.
@Shrinks99
Readme updates for fonts
8875925 
Also a few punctuation fixes and inclusion of Work Sans in the credits.
@Shrinks99 Shrinks99 added the Approval Requested Request for approval, otherwise ready to go label Dec 15, 2018
garrying
garrying approved these changes Dec 21, 2018
View reviewed changes
Copy link
Member

@garrying garrying left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔Stray thought: Might be helpful to describe how a project folder could be structured—having a /fonts folder per-project might be helpful for projects that use other fonts.

@Shrinks99
Copy link
Member Author

That's not a bad idea @garrying, my thought with a single folder is that it would reduce the duplication of files. Having a way to reference what fonts are needed in a given project is valuable though, perhaps a readme?

@dcwalk
Copy link
Contributor

dcwalk commented Dec 21, 2018

My two cents: I agree about a per-folder fonts subfolder... often preferred to have all needed files contained within a project folder to minimize not realizing there is a missing vendor/src file

@benhylau
Copy link
Member

Agree with @dcwalk about depending on things only inside a folder if possible. Once you start depending on ../ it is harder to remix things modularly.

This of course has issue of duplication which usually has two drawbacks:

  1. Total file size becomes larger
  2. If we update a version of the asset it needs to be updated in multiple places

This is a design repo, I don't really care about 1. As for 2, we actually want updates to apply modularly, instead of having to update all assets together. So given context, I think per-folder fonts is a better choice here.

benhylau
benhylau requested changes Dec 24, 2018
View reviewed changes
Copy link
Member

@benhylau benhylau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do per-folder fonts. I think it's important we start that structure before it gets out of control :)

@Shrinks99
Copy link
Member Author

General consensus seems to be per-folder fonts. I'll make the changes!

@dcwalk dcwalk removed their request for review February 3, 2021 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benhylau benhylau benhylau requested changes

@garrying garrying garrying approved these changes

@dasanchez dasanchez Awaiting requested review from dasanchez

Assignees
No one assigned
Labels
Approval Requested Request for approval, otherwise ready to go
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Shrinks99 @dcwalk @benhylau @garrying